Redis New Implementation #585
Conversation
pkg/redis/RedisConsumer.php
Outdated
| /** | ||
| * @return int | ||
| */ | ||
| public function getRetryDelay(): ?int |
pkg/redis/RedisQueueConsumer.php
Outdated
| * @param Redis $redis | ||
| * @param RedisDestination[] $queues | ||
| */ | ||
| public function __construct(Redis $redis, array $queues) |
There was a problem hiding this comment.
I'd move array $queues to receiveXXX methods. I think it would be better to make it as stateless as possible
pkg/redis/RedisQueueConsumer.php
Outdated
| */ | ||
| public function getRetryDelay(): int | ||
| { | ||
| return $this->retryDelay; |
There was a problem hiding this comment.
could we pass it as an argument of receiveXXX methods? S
pkg/redis/RedisQueueConsumer.php
Outdated
|
|
||
| namespace Enqueue\Redis; | ||
|
|
||
| class RedisQueueConsumer |
There was a problem hiding this comment.
The name is misleading IMO. Could it be named RedisInternalConsumer or RedisGenericConsumer.
pkg/redis/RedisConsumer.php
Outdated
|
|
||
| private function initQueueConsumer(): void | ||
| { | ||
| if (null === $this->queueConsumer) { |
There was a problem hiding this comment.
if we make a RedisQueueConsumer stateless we could move it to the context and pass to real consumers as a dependency.
pkg/redis/RedisConsumer.php
Outdated
| { | ||
| $this->retryDelay = $retryDelay; | ||
|
|
||
| if ($this->queueConsumer) { |
There was a problem hiding this comment.
I think setting retryDelay property is enough. No need to set same value on queue consumer too. (It will be passed as argument to receiveXXX methods).
pkg/redis/RedisQueueConsumer.php
Outdated
|
|
||
| $now = time(); | ||
|
|
||
| if ($expiresAt = $message->getHeader('expires_at')) { |
There was a problem hiding this comment.
If we managed to processes a message before expiration time, but failed and as a result the message was redelivered, and this condition reject redelivered message as expired. I dont think this is desired behavior.
pkg/redis/RedisQueueConsumer.php
Outdated
| return $message; | ||
| } | ||
|
|
||
| private function pushQueueNameBack($queueName): void |
pkg/redis/RedisProducer.php
Outdated
| @@ -34,24 +45,44 @@ public function send(Destination $destination, Message $message): void | |||
| InvalidDestinationException::assertDestinationInstanceOf($destination, RedisDestination::class); | |||
| InvalidMessageException::assertMessageInstanceOf($message, RedisMessage::class); | |||
|
|
|||
| $this->redis->lpush($destination->getName(), json_encode($message)); | |||
| $message->setMessageId(UUID::generate()); | |||
There was a problem hiding this comment.
require ramnsy uuid and use it.
Supports